Skip to content

Fix .. collapsing with other .. during AssetPath normalization. #21876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix .. collapsing with other .. during AssetPath normalization. #21876

wants to merge 3 commits into from
+25 −1

Conversation

@andriyDev
Copy link
Contributor

@andriyDev andriyDev commented Nov 18, 2025

Objective

Solution

  • Make sure that we're not collapsing .. with itself!

Testing

  • Added a test for this. It failed but after the fix it passes!

@andriyDev andriyDev added this to the 0.17.4 milestone Nov 18, 2025
@andriyDev andriyDev added C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy A-Assets Load files from disk to use for things like images, models, and sounds S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 18, 2025
DGriffin91
DGriffin91 reviewed Nov 18, 2025
View reviewed changes
@DGriffin91
Copy link
Contributor

DGriffin91 commented Nov 18, 2025

Tried this on my repro from the issue and it seemed to work.

@andriyDev andriyDev requested a review from DGriffin91 November 18, 2025 03:16
@andriyDev andriyDev added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 18, 2025
it-me-joda
it-me-joda approved these changes Nov 20, 2025
View reviewed changes
Copy link
Contributor

@it-me-joda it-me-joda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@it-me-joda it-me-joda it-me-joda approved these changes

@DGriffin91 DGriffin91 DGriffin91 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Milestone

0.17.4

Development

Successfully merging this pull request may close these issues.

Assets from relative paths outside the asset folder not loading

3 participants

@andriyDev @DGriffin91 @it-me-joda